New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: disallow all parenthesized pattern except parsing LHS #12327
fix: disallow all parenthesized pattern except parsing LHS #12327
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/32292/ |
let parenthesized = undefined; | ||
if (node.type === "ParenthesizedExpression" || node.extra?.parenthesized) { | ||
parenthesized = unwrapParenthesizedExpression(node); | ||
if ( | ||
parenthesized.type !== "Identifier" && | ||
parenthesized.type !== "MemberExpression" | ||
!isLHS || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My first attempt is to use !this.state.maybeInArrowParameters
here but I realize that then we have to reset this state for class blocks. It is easy to mess up when things get nested.
(a = class { static { [(a)] = 1 }) => {}
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 46de321:
|
@@ -891,11 +884,6 @@ export default class ExpressionParser extends LValParser { | |||
); | |||
} | |||
|
|||
// we found an async arrow function so let's not allow any inner parens | |||
if (possibleAsyncArrow && innerParenStart && this.shouldParseAsyncArrow()) { | |||
this.unexpected(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of throwing unexpected errors on
var foo = async ((foo)) => {};
we can now recover and raise this error in toAssignable
, which I think is more helpful
"SyntaxError: Invalid parenthesized assignment pattern (1:18)"
76b8566
to
4eecb1b
Compare
@@ -1421,12 +1409,6 @@ export default class ExpressionParser extends LValParser { | |||
) { | |||
this.expressionScope.validateAsPattern(); | |||
this.expressionScope.exit(); | |||
for (const param of exprList) { | |||
if (param.extra && param.extra.parenthesized) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can now recover from var foo = ((foo)) => {};
@@ -1 +0,0 @@ | |||
((a)) => 42 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is removed because it is almost identical to
babel/packages/babel-parser/test/fixtures/es2015/arrow-functions/inner-parens/input.js
Line 1 in 3227914
var foo = ((foo)) => {}; |
@@ -1 +0,0 @@ | |||
((a)) => 42 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is removed because it is almost identical to
babel/packages/babel-parser/test/fixtures/es2015/arrow-functions/inner-parens/input.js
Line 1 in 3227914
var foo = ((foo)) => {}; |
@@ -1 +0,0 @@ | |||
(a, (b)) => 42 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is removed because it is identical to
babel/packages/babel-parser/test/fixtures/es2015/arrow-functions/inner-parens-2/input.js
Line 1 in 4eecb1b
(a, (b)) => 42 |
4eecb1b
to
0c1c38e
Compare
|
||
* @returns {Node} The converted assignable pattern | ||
* @memberof LValParser | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all the doc comments you are introducing!
...ges/babel-parser/test/fixtures/es2015/arrow-functions/.inner-parens-array-pattern-2/input.js
Outdated
Show resolved
Hide resolved
...ages/babel-parser/test/fixtures/es2015/arrow-functions/.inner-parens-object-pattern/input.js
Outdated
Show resolved
Hide resolved
9429966
to
00a776c
Compare
@@ -1 +0,0 @@ | |||
([ [(a)] = [] ] = []) => {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have enabled these tests on 00a776c but forgot to remove the skipped ones: https://github.com/babel/babel/pull/12327/files#diff-cb581cc9c63ffb0abb517f0508ad77cfd53fa50f53f16ee6e61eda11a0be0e3d
cc6569c
to
64b9236
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
Co-authored-by: Brian Ng <bng412@gmail.com>
([(a)]) => {}
andasync ([(a)]) => {}
(REPL)Found this bug when skimming on parser source and reading on
My gut tells me the current implementation is not complete and it turns out it can not handle edge cases like I mentioned above. I come up with current solution when jogging in the warm early November.